Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

perf($compile, jqLite): reduce creation of unnecessary jquery objects #7963

Closed
wants to merge 6 commits into from

Conversation

jbedard
Copy link
Collaborator

@jbedard jbedard commented Jun 24, 2014

This removes the creation of many duplicate or unnecessary jQuery objects in the compile/link phases as well as in some jqLite methods.

Most of the changes are pretty minor and safe (as far as myself and the tests can tell).

The only bigger change is exposing jqLiteData/jqLiteRemoveData as jqLite.data/removeData to match the jQuery equivalent (https://api.jquery.com/jquery.data/). This allows getting/setting data without creating the jQuery wrapper object and allows for the biggest reduction in jQuery objects in compile/link. This also reduces the jqLite objects created in the scope/isolatedScope/data/inheritedData methods and simplifies jqLiteInheritedData. Note that this change overrides the first commit.

If interested I can squash all the commits and maybe add more tests for the jqLite changes.

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7963)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@jbedard jbedard added cla: yes and removed cla: no labels Jun 24, 2014
jbedard added 2 commits June 27, 2014 00:05
- updated the internal jqLite helpers to use the low-level jqLite.data/removeData to avoid unnecessary jq wrappers and loops
- updated $compile to use the low-level jqLite.data/removeData to avoid unnecessary jq wrappers at link time
…educe-jquery-objects

Conflicts:
	test/jqLiteSpec.js
@rodyhaddad rodyhaddad added this to the 1.3.0 milestone Jun 29, 2014
@rodyhaddad
Copy link
Contributor

LGTM

@IgorMinar
Copy link
Contributor

this is good stuff. I reviewed the commits and they all look good to me.

@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.16, 1.3.0 Jul 15, 2014
@IgorMinar
Copy link
Contributor

@rodyhaddad can you get this in and backport to 1.2.x please?

@rodyhaddad rodyhaddad mentioned this pull request Jul 18, 2014
@rodyhaddad
Copy link
Contributor

Rebased, squashed and landed
Master: a160f76, e4ba894
v1.2.x: 71eb190, 3c46c94

@rodyhaddad rodyhaddad closed this Jul 18, 2014
@jbedard
Copy link
Collaborator Author

jbedard commented Jul 18, 2014

Awesome, thanks for squashing those and getting them in!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants